escape colon in path segment #1237
Conversation
lib/faraday/connection.rb
Outdated
| uri = url ? base + url : base | ||
| uri = URI.parse(CGI.unescape(uri.to_s)) |
There was a problem hiding this comment.
Not sure whether final uri should be unescaped
There was a problem hiding this comment.
That's a good point, this may cause issues in case the URL contains escaped characters like spaces? Escaped columns may also be part of the URL, so I'm not sure unescaping here is a good idea at all.
E.g. Faraday.get('https://service.com/path%20with%20spaces')
There was a problem hiding this comment.
I like the approach and even more the test coverage! But unfortunately the escaping solution might not be the best one 🤔. I'm curious to see if other tests are passing (I'd expect at least one to fail, if we're testing spaces!), but Rubocop is in the way...
The ClassLength cop is a tricky one, so feel free to increase the limit in the .rubocop_todo.yml file for this time. Since you added 2 lines that should be 236 now, I guess?
lib/faraday/connection.rb
Outdated
| uri = url ? base + url : base | ||
| uri = URI.parse(CGI.unescape(uri.to_s)) |
There was a problem hiding this comment.
That's a good point, this may cause issues in case the URL contains escaped characters like spaces? Escaped columns may also be part of the URL, so I'm not sure unescaping here is a good idea at all.
E.g. Faraday.get('https://service.com/path%20with%20spaces')
|
@yarafan this looks good now, but is your service call working as expected even with the |
|
@iMacTia I don't know any service with such uri. I tried to access uri in issue and got 302. |
|
Hey guys! I can confirm it works and this is how the URL appeared on the logs INFO Rails : request: POST https://subdomain.service.com/api/loans%3Asearch?limit=50&offset=2900 |
Description
Considering example
URI.parseparses relative pathservice:searchas opaque_part (according to https://tools.ietf.org/html/rfc2396#section-3) (egmailto:foo@example.com), soFaraday::Connection#build_exclusive_urlproduces wrong URI as result and passing query params leads toInvalidURIErrorFixes #1214
Todos
List any remaining work that needs to be done, i.e: